-
Notifications
You must be signed in to change notification settings - Fork 319
Add request and challenge callbacks to BearerTokenAuthorizationPolicy #3368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
1d5a6cd to
4785d13
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds extensible request authorization and authentication challenge handling to BearerTokenAuthorizationPolicy, enabling SDK clients to implement custom authorization and challenge handling logic.
- Introduces three new public traits:
OnRequest,OnChallenge, andAuthorizer(sealed) to define callbacks for request authorization and challenge handling - Adds builder methods
with_on_request()andwith_on_challenge()to configure policy behavior - Adds
Request::body_mut()method to support resetting body streams when retrying after challenges - Adds
WWW_AUTHENTICATEheader constant for accessing authentication challenge headers
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| sdk/core/typespec_client_core/src/http/request/mod.rs | Adds body_mut() method to enable mutable access to request body for stream reset operations |
| sdk/core/typespec_client_core/src/http/headers/common.rs | Adds WWW_AUTHENTICATE header constant following existing header constant patterns |
| sdk/core/typespec_client_core/CHANGELOG.md | Documents the Request::body_mut() API addition |
| sdk/core/azure_core/src/http/policies/mod.rs | Exports new public traits (Authorizer, OnChallenge, OnRequest) from bearer_token_policy module |
| sdk/core/azure_core/src/http/policies/bearer_token_policy.rs | Refactors authorization logic into BearerTokenAuthorizer implementing Authorizer trait; adds OnRequest and OnChallenge trait definitions; implements challenge handling with body stream reset; adds comprehensive test coverage for new functionality |
| sdk/core/azure_core/CHANGELOG.md | Documents new extensible authorization and challenge handling features |
heaths
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, clean design overall but I have a few questions and concerns.
sdk/core/azure_core/src/http/policies/auth/bearer_token_policy.rs
Outdated
Show resolved
Hide resolved
7794584 to
59806ed
Compare
heaths
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebase approved.
...so it can pass request-specific data along to on_challenge
These enable SDK clients to implement custom request authorization and challenge handling (closes #1756).
New API:
OnRequesttrait: defines a callback invoked on each request, responsible for authorizing it before sendingOnChallengetrait: defines a callback invoked upon receiving an auth challenge. Implementations are responsible for parsing challenges and authorizing requests and telling the policy whether to retry themBearerTokenAuthorizationPolicybuilder methods:with_on_request()andwith_on_challenge()set callbacks for a policy instanceAuthorizertrait (sealed): allows callbacks to authorize requests and cache tokens without using the policy's credential directlysend()into a private implementation of thisRequest::body_mut(): lets the policy reset a body stream before retrying after a challengeI put all this in a new module
azure_core::http::policies::authand movedBearerTokenAuthorizationPolicythere as well.